Add NGTS configuration + NGTS client by SgtCoDFish · Pull Request #788 · jetstack/jetstack-secure

SgtCoDFish · 2026-03-24T12:34:45Z

This will add initial support for NGTS. Auth is based on the existing Venafi Cloud client using a keypair.

I'm not really able to test this effectively because of various issues with the test env, but I think this is safe enough to merge as-is because it's not customer-facing yet (needs helm chart support before this is realistically usable)

Note there are several TODOs in this PR. They need to be clarified before we can expose this functionality to customers, but I think they're fine for now.

SgtCoDFish · 2026-03-26T13:17:56Z

pkg/client/client_ngts.go

NB: this is pretty much copy+pasted from client_venafi_cloud.go because the logic is nearly identical. I refactored some of the shared logic out (util.go) but mostly this is the same thing with different names

This will add initial support for NGTS. Auth is based on the existing Venafi Cloud client using a keypair. Signed-off-by: Ashley Davis <ashley.davis@cyberark.com>

pkg/client/client_ngts.go

George-Yanev · 2026-04-02T12:28:04Z

pkg/agent/config.go

+		if res.OutputMode == NGTS && res.NGTSServerURL != "" {
+			log.Info("Using custom NGTS server URL (for testing)", "url", res.NGTSServerURL)
+			server = res.NGTSServerURL
 		}


propose (non-blocking): The final result will be the same and in the age of AI I'm not sure if anybody will look at this but having it written that way looks more readable to me.

Feel free to disregard it

if res.OutputMode == NGTS { if res.NGTSServerURL != "" { log.Info("Using custom NGTS server URL (for testing)", "url", res.NGTSServerURL) } else if cfg.Server != "" { log.Info(fmt.Sprintf("ignoring the server field in the config file. In %s mode, use --ngts-server-url for testing.", NGTS)) } server = res.NGTSServerURL }

George-Yanev · 2026-04-02T13:54:51Z

pkg/client/client_ngts.go

+		return fmt.Errorf("failed to obtain NGTS access token: %w", err)
+	}
+
+	c.lock.Lock()


question (non-blocking): We have a write lock but the code is actually sequential and we never do the read lock in the getValidAccessToken. I guess this was a legacy. Just pointing it out.

George-Yanev · 2026-04-02T14:04:21Z

pkg/agent/config.go

+			if res.OutputMode == NGTS {
+				// In NGTS mode, use NGTSServerURL if provided, otherwise we'll use a default
+				// (which will be determined when creating the client)
+				server = res.NGTSServerURL
+			}
+		}
+
+		// For NGTS mode with custom server URL
+		if res.OutputMode == NGTS && res.NGTSServerURL != "" {
+			log.Info("Using custom NGTS server URL (for testing)", "url", res.NGTSServerURL)
+			server = res.NGTSServerURL
 		}
+
 		url, urlErr := url.Parse(server)
-		if urlErr != nil || url.Hostname() == "" {
+		if urlErr != nil || (url.Hostname() == "" && server != "") {
 			errs = multierror.Append(errs, fmt.Errorf("server %q is not a valid URL", server))
 		}


propose (non-blocking): Doing the same but IMHO more readable

// In NGTS mode: ignore the config-file server field entirely; use only // --ngts-server-url when provided (default URL is derived from TSG ID // at client construction time). if res.OutputMode == NGTS { if res.NGTSServerURL != "" { log.Info("Using custom NGTS server URL (for testing)", "url", res.NGTSServerURL) } else if cfg.Server != "" { log.Info(fmt.Sprintf("ignoring the server field in the config file. In %s mode, use --ngts-server-url for testing.", NGTS)) } server = res.NGTSServerURL } url, urlErr := url.Parse(server) // An empty server is valid: NGTS derives its URL from the TSG ID at client // creation; VenafiCloudVenafiConnection doesn't use one at all. if server != "" && (urlErr != nil || url.Hostname() == "") { errs = multierror.Append(errs, fmt.Errorf("server %q is not a valid URL", server)) }

George-Yanev · 2026-04-02T14:18:15Z

pkg/client/client_ngts.go

+	ok, why := credentials.IsClientSet()
+	if !ok {
+		return nil, fmt.Errorf("%s", why)
+	}


propose (non-blocking): Doesn't the IsClientSet and Validate on line 125 do the same. From what I'm seeing they do. If that's true does it make sense to remove the IsClientSet?

George-Yanev · 2026-04-02T14:19:06Z

pkg/client/client_ngts.go

+			extra = fmt.Sprintf(" (possibly malformed TSG ID %q?)", tsgID)
+		}
+
+		return nil, fmt.Errorf("invalid SCM base URL %q: %s%s", baseURL, err, extra)


proposal: Shouldn't that be NGTS instead of SCM?

inteon · 2026-04-03T08:25:14Z

pkg/client/client_ngts.go

+	claims["iss"] = c.credentials.ClientID
+	claims["iat"] = time.Now().Unix()
+	claims["exp"] = time.Now().Add(time.Minute).Unix()
+	claims["aud"] = path.Join(c.baseURL.Host, ngtsAccessTokenEndpoint)


I think the NGTS endpoint has not been updated yet to accept anything other than "api.venafi.cloud/v1/oauth/token/serviceaccount" here?

Is this working for the NGTS endpoint?

achuchev · 2026-04-03T08:10:54Z

pkg/client/util.go

+func parsePrivateKeyFromPEMFile(privateKeyFilePath string) (crypto.PrivateKey, error) {
+	pkBytes, err := os.ReadFile(privateKeyFilePath)
+	if err != nil {
+		return nil, fmt.Errorf("failed to fetch Venafi Cloud authentication private key %q: %s",


Suggested change

return nil, fmt.Errorf("failed to fetch Venafi Cloud authentication private key %q: %s",

return nil, fmt.Errorf("unable to read private key for authentication %q: %s",

Because this logic applies to both Venafi Cloud and NGTS.

achuchev · 2026-04-03T08:17:09Z

pkg/client/client_ngts.go

+	uploadURL.RawQuery = query.Encode()
+
+	klog.FromContext(ctx).V(2).Info(
+		"uploading data readings to SCM",


Suggested change

"uploading data readings to SCM",

"uploading data readings to NGTS",

Let's make it consistent

achuchev · 2026-04-03T08:21:18Z

pkg/agent/config.go

+		&cfg.TSGID,
+		"tsg-id",
+		"",
+		"The TSG (Tenant Security Group) ID for NGTS mode. Required when using --ngts.",


Suggested change

"The TSG (Tenant Security Group) ID for NGTS mode. Required when using --ngts.",

"The TSG (Tenant Service Group) ID for NGTS mode. Required when using --ngts.",

achuchev · 2026-04-03T08:22:16Z

pkg/agent/config.go

+	// using key pair authentication and send data to NGTS endpoints.
+	NGTSMode bool
+
+	// TSGID (--tsg-id) is the TSG (Tenant Security Group) ID for NGTS mode.


Suggested change

// TSGID (--tsg-id) is the TSG (Tenant Security Group) ID for NGTS mode.

// TSGID (--tsg-id) is the TSG (Tenant Service Group) ID for NGTS mode.

achuchev · 2026-04-03T08:22:35Z

pkg/client/client_ngts.go

+}
+
+// PostDataReadingsWithOptions uploads data readings to the NGTS backend.
+// The TSG ID is included in the upload path to identify the tenant security group.


Suggested change

// The TSG ID is included in the upload path to identify the tenant security group.

// The TSG ID is included in the upload path to identify the tenant service group.

achuchev · 2026-04-03T08:26:09Z

pkg/client/client_ngts.go

+		return nil, fmt.Errorf("cannot create NGTSClient: %w", err)
+	}
+
+	if tsgID == "" {


@SgtCoDFish Any additional validation that makes sense to be done at this point? I am pretty sure that we will have something in the Helm chart.
FYI, according to the link below, TSG ID is

Possible values: >= 10 characters and <= 10 characters, Value must match regular expression ^1[0-9]+$

https://pan.dev/scm/api/tenancy/delete-tenancy-v-1-tenant-service-groups-tsg-id/

SgtCoDFish force-pushed the ngts-flags branch from 8b8a917 to b1303d8 Compare March 24, 2026 14:29

SgtCoDFish commented Mar 26, 2026

View reviewed changes

add config + new client for NGTS

6b996d5

This will add initial support for NGTS. Auth is based on the existing Venafi Cloud client using a keypair. Signed-off-by: Ashley Davis <ashley.davis@cyberark.com>

SgtCoDFish force-pushed the ngts-flags branch from b1303d8 to 6b996d5 Compare March 26, 2026 13:34

George-Yanev self-assigned this Apr 1, 2026

inteon reviewed Apr 1, 2026

View reviewed changes

pkg/client/client_ngts.go Show resolved Hide resolved

George-Yanev approved these changes Apr 2, 2026

View reviewed changes

inteon reviewed Apr 3, 2026

View reviewed changes

achuchev approved these changes Apr 3, 2026

View reviewed changes

	return nil, fmt.Errorf("failed to fetch Venafi Cloud authentication private key %q: %s",
	return nil, fmt.Errorf("unable to read private key for authentication %q: %s",

	"uploading data readings to SCM",
	"uploading data readings to NGTS",

	"The TSG (Tenant Security Group) ID for NGTS mode. Required when using --ngts.",
	"The TSG (Tenant Service Group) ID for NGTS mode. Required when using --ngts.",

	// TSGID (--tsg-id) is the TSG (Tenant Security Group) ID for NGTS mode.
	// TSGID (--tsg-id) is the TSG (Tenant Service Group) ID for NGTS mode.

	// The TSG ID is included in the upload path to identify the tenant security group.
	// The TSG ID is included in the upload path to identify the tenant service group.

Conversation

SgtCoDFish commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

George-Yanev Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

SgtCoDFish commented Mar 24, 2026 •

edited

Loading

George-Yanev Apr 2, 2026 •

edited

Loading